Skip to content

fix(bitbucket): PR sync stops resending diffstat, conflicts, statuses and permission every sweep - #27596

Open
juliusmarminge wants to merge 7 commits into
mainfrom
fix/issue-27578
Open

juliusmarminge wants to merge 7 commits into
mainfrom
fix/issue-27578

Conversation

@juliusmarminge

Copy link
Copy Markdown
Member

Problem

Pull request sync sent 5 Bitbucket API requests for every linked pull request on every sweep: the pull request, diffstat, conflicts, statuses and repository permission. Bitbucket has no getChangeRequestSummary, so the sync summary read fell back to the full detail read, including statuses and permission, which a summary never uses. With 12 recently updated pull requests on the one-minute sweep that is about 3,600 requests an hour. Bitbucket allows 1,000, so the host stays paused and every Bitbucket feature fails.

Reproduced on main with the real dev server, pointed through T3CODE_BITBUCKET_API_BASE_URL at a local fake Bitbucket API that logs requests. With 12 linked pull requests, every sweep sent 60 requests.

Change

The Bitbucket provider now has its own getChangeRequestSummary. It reads the pull request on every sweep. It reuses diffstat and conflicts while the pull request's source and destination commits are unchanged, and never requests statuses or repository permission. The summary carries the same fields as before.

  • What is held. Diffstat and conflicts, per pull request and commit pair, for at most 15 minutes. The time limit covers a destination branch that moves without the pull request's destination commit following it.
  • What drops a held read early. A refresh or a finished turn, through forgetChangeRequestReads.
  • What is never held. Failed reads, and a conflicts read that failed and was recovered as unknown. A 429 or the host-wide pause still fails the read as rate-limited.
  • No commit hash. A pull request without either hash reads diffstat and conflicts directly.

The detail panel's getChangeRequest is unchanged.

Scope and approval

Fixes #27578

Verification

  • vp test run packages/source-control-bitbucket: 6 files, 92 tests pass.
    • rereads only the pull request when sync reads an unchanged pull request again fails on main: the first and second reads each also call conflicts, diffstat, permission and statuses.
    • rereads diffstat and conflicts when a commit moves, the reads expire or are forgotten, or conflicts failed covers a destination commit move, a source commit move, a forget with a differently cased repository, the 15-minute expiry (TestClock), and a failed conflicts read not being held. With the expiry removed it fails, 6 reads instead of 7.
  • vp run check:changed: passes.
  • Runtime transcript, identical setup on both sides:
    • The real dev server (T3CODE_DEV_SEED=0 vp run dev --no-seed, fresh database), with the fake Bitbucket API logging every request.
    • 12 synthetic pull requests linked to 4 threads through the server's MCP link_pull_request, recently updated so they are read every sweep.
    • The first line of each side is the link-time read.
== Before: main 32ecd71726 ==
12:39:31 UTC  requests= 60  pull_requests=12  conflicts=12 diffstat=12 permission=12 pullrequest=12 statuses=12
12:41:29 UTC  requests= 60  pull_requests=12  conflicts=12 diffstat=12 permission=12 pullrequest=12 statuses=12
12:42:29 UTC  requests= 60  pull_requests=12  conflicts=12 diffstat=12 permission=12 pullrequest=12 statuses=12
12:43:30 UTC  requests= 60  pull_requests=12  conflicts=12 diffstat=12 permission=12 pullrequest=12 statuses=12
12:44:30 UTC  requests= 60  pull_requests=12  conflicts=12 diffstat=12 permission=12 pullrequest=12 statuses=12

== After: fix/issue-27578 a5fc36dcfb ==
12:54:04 UTC  requests= 36  pull_requests=12  conflicts=12 diffstat=12 pullrequest=12
12:56:01 UTC  requests= 12  pull_requests=12  pullrequest=12
12:57:02 UTC  requests= 12  pull_requests=12  pullrequest=12
12:58:02 UTC  requests= 14  pull_requests=12  conflicts=1 diffstat=1 pullrequest=12   (new source commit pushed to #1 at 12:57:09)
12:59:02 UTC  requests= 12  pull_requests=12  pullrequest=12
13:00:02 UTC  requests= 12  pull_requests=12  pullrequest=12

Steady state drops from 60 requests a sweep to 12, about 720 an hour. The push rereads diffstat and conflicts for that pull request only.

  • Not checked:
    • Real Bitbucket Cloud, since no account is available. The fake API serves the same JSON shapes the decoders read.
    • The fake API, seed driver and request logs are not attached: the upload tool was unavailable on the machine that ran them.

Model and harness: Claude Opus 5.5 (Claude Code) as issue lead, red test and implementation; reviews by GPT-6.1 Sol (Codex) and Claude Opus 5.5.

🤖 Generated with Claude Code

juliusmarminge and others added 4 commits October 30, 2026 12:30
…a (reproduces #27578)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ts are unchanged

Bitbucket had no summary read, so every sync sweep ran the full detail read
for each pull request: the pull request, diffstat, conflicts, statuses and
repository permission, though the summary uses neither statuses nor
permission.

Add `getChangeRequestSummary`, which reads the pull request and reuses the
diffstat and conflicts held for the same source and destination commits.
Reads are not held when either commit hash is missing, when they fail, or
when the conflicts read fell back to `unknown`. `forgetChangeRequestReads`
drops them on a refresh or a finished turn. The detail read is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Bitbucket may leave a pull request's destination commit in place while the
destination branch moves, so conflicts keyed on the two commits could stay
stale until a refresh. The test also covers a recovered conflicts failure,
which is never held.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 30, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at a58901b

Macroscope's review found this PR approvable — Bitbucket sync avoids redundant API requests by caching commit-dependent summary data while leaving full detail reads unchanged. Commit-based keys, a 15-minute expiry and refresh invalidation limit stale summaries without changing permissions or merge behavior.

No code changes detected at f19ef1d. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 21.5 KiB 21.5 KiB 0 B (0.0%) 24.4 KiB ✅
Codex Live turn messages 1 1 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.9 KiB 21.9 KiB 0 B (0.0%) 24.4 KiB ✅
Claude Live turn messages 1 1 0 (0.0%) 8 ✅

Baseline: ba32aab · PR result: f19ef1d · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@coderabbitai

coderabbitai Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 027cc88 and f19ef1d.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review

No actionable comments were generated in the recent review.

Walkthrough

Bitbucket pull-request summaries now reuse diff-stat and mergeability results when both commit SHAs are available. The parsed pull-request model now includes the trimmed destination commit SHA as baseSha.

Changes: Bitbucket summary cache

Layer / File(s) Summary
Commit-keyed summary cache and invalidation
packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts, packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts, packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts
The pull-request data includes a trimmed destination commit SHA as baseSha. The provider caches diff statistics and mergeability by repository, pull-request number, and both commit SHAs. The cache has a 256-entry limit and a 15-minute TTL for successful reads with mergeability data. The provider reads values directly when either SHA is unavailable. Cache invalidation can clear all entries or match a repository and optional pull-request number. Tests cover reuse, changed commits, invalidation, expiration, and mergeability read failures.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: High

Sequence Diagram(s)

sequenceDiagram
  participant Provider as BitbucketPullRequestProvider
  participant API as Bitbucket API
  participant Cache as Summary cache
  Provider->>API: Fetch pull request and commit SHAs
  Provider->>Cache: Look up by repository, pull request, and SHAs
  alt Cache hit
    Cache-->>Provider: Return diff statistics and mergeability
  else Cache miss
    Provider->>API: Read diff statistics and mergeability
    API-->>Provider: Return summary data
    Provider->>Cache: Store result with expiration
  end
Loading

Possibly related PRs

  • pingdotgg/t3code#27100: Reuses pull-request summary reads for unchanged commits in the separate GitCafe provider.

Suggested reviewers: maria-rcks, ahilles107

Merge Risk: Minimal · previous review fd237

The summary path appears to reduce repeat Bitbucket reads without breaking the sync snapshot. No identified issue needs resolution before normal merge checks.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly and concisely describes the main Bitbucket sync change: preventing repeated diffstat, conflict, status, and permission requests.
Description check Passed The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the rate-limit issue, implementation, linked issue, tests, runtime results, and limitation…
Linked Issues check Passed [#27578] The Bitbucket summary path now performs one pull-request read per sync and does not request statuses or repository permission. It caches diffstat and conflicts by repository, pull-request num…
Out of Scope Changes check Passed The changes stay within [#27578]. The provider adds the summary read, bounded commit-stat caching, and invalidation needed for sync. The JSON model adds the destination commit hash needed for the cach…
Full details: Description check

Explanation

The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the rate-limit issue, implementation, linked issue, tests, runtime results, and limitations.

Full details: Linked Issues check

Explanation

[#27578] The Bitbucket summary path now performs one pull-request read per sync and does not request statuses or repository permission. It caches diffstat and conflicts by repository, pull-request number, source commit, and destination commit. It rereads those values after a commit change, cache expiry, explicit invalidation, or an unrecovered conflicts read. The added tests cover unchanged reads, commit changes, expiry, invalidation, and failed conflicts reads. The reported package tests and changed-file checks pass. The implementation uses one pull-request read per linked pull request instead of the issue's optional list-endpoint batching, but it meets the request-reduction objective.

Full details: Out of Scope Changes check

Explanation

The changes stay within [#27578]. The provider adds the summary read, bounded commit-stat caching, and invalidation needed for sync. The JSON model adds the destination commit hash needed for the cache key. The tests verify the new sync behavior. No unrelated product behavior appears in the reviewed diff.

Finishing Touches

Generate docstrings

  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between a58901b and fd23744.

Files selected for processing: 2

  • packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts
  • packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.ts

Limit details: You’ve used all 10 included reviews currently available.

@aravhawk aravhawk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question about the failed-conflicts path in getChangeRequestSummary.

readCommitStats reads diffstat and conflicts together, and the cache's timeToLive returns Duration.zero whenever exit.value[1] === null. recoverRead turns every non-429 error from /conflicts into that null: 403, 404, decode errors, request errors and so on. So if a PR's conflicts read fails every time instead of once, its entry is never held. Sync then goes back to pull request + diffstat (every page) + conflicts on each sweep for that PR, which is 3+ requests instead of 1, and the diffstat that did succeed gets thrown away as well.

Can /conflicts fail persistently on Bitbucket Cloud? For example, merged or declined PRs that sync still reads because they were recently updated. If it can, would it be worth caching diffstat under its own key and leaving only the conflicts retry uncached? Or holding a null for a short TTL (a minute or two), so it still retries without paying for diffstat on every sweep? The new test only covers a single 503 that recovers on the next read, so it wouldn't catch the persistent case either way.

@juliusmarminge juliusmarminge added bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed labels Nov 9, 2026
…e-27578

# Conflicts:
#	packages/source-control-bitbucket/src/server/BitbucketPullRequestProvider.test.ts

Copy link
Copy Markdown
Member Author

The automated pipeline stopped on this pull request and will not merge it:

someone else pushed to fix/issue-27578 (head is now a58901b); the pipeline does not build on commits it did not review

It stays open for a person to finish or close.

@juliusmarminge

Copy link
Copy Markdown
Member Author

The automated pipeline stopped on this pull request and will not merge it:

the proof has no capture anyone can open:

  • The base's reported 60 requests per sweep and the head's expected 36 initial and 12 subsequent requests: authenticated Bitbucket access was unavailable.
  • Refresh after source-commit changes, cache expiry, and explicit invalidation: authenticated Bitbucket access was unavailable.
  • No public capture URLs exist because no runtime reproduction was possible; ghdrop upload was not attempted.

It stays open for a person to finish or close.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fix PR opened by an issue lead; enters the PR loop pr:gate-failed A land gate failed size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Bitbucket sends 5 requests per pull request on every sync

2 participants